Skip to content

[tests] Add tests for skip/enable-plugins merge from config and cmdline#4358

Open
jcastill wants to merge 1 commit into
sosreport:mainfrom
jcastill:jcastillo-add-test-4327
Open

[tests] Add tests for skip/enable-plugins merge from config and cmdline#4358
jcastill wants to merge 1 commit into
sosreport:mainfrom
jcastill:jcastillo-add-test-4327

Conversation

@jcastill

@jcastill jcastill commented Jun 4, 2026

Copy link
Copy Markdown
Member

Verify that skip-plugins and enable-plugins are merged (not replaced)
when specified in both sos.conf and command line.

This is a regression test for commit 8fc655a, which fixed the merge
behavior. The new tests verify both effective options
logging in sos.log and actual plugin execution.

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

  • Is the commit message split over multiple lines and hard-wrapped at 72 characters?
  • Is the subject and message clear and concise?
  • Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
  • Does the commit contain a Signed-off-by: First Lastname email@example.com?
  • Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
  • Are all passwords or private data gathered by this PR obfuscated?

@jcastill jcastill added the Kind/Testing Related to Testing label Jun 4, 2026
@jcastill

jcastill commented Jun 4, 2026

Copy link
Copy Markdown
Member Author

Some quick notes and questions:

  • I based the *_sos.conf files on the one we have for report_tests/option_tests, but I can clean them up by removing the commented options and leaving just what's needed for the tests.
  • I have my doubts about the test location. I have them live inside option_tests because of the cmdline options, but then I decided to move them to a specific new location inside plugins. Does this location make sense?
  • Any improvements regarding test names are welcome, as always :)
  • Considered adding a test for preset vs config file vs cmdline, to make it more complete. Thoughts on this?

@packit-as-a-service

packit-as-a-service Bot commented Jun 4, 2026

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/sosreport-sos-4358
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@arif-ali

arif-ali commented Jun 4, 2026

Copy link
Copy Markdown
Member
  • I based the *_sos.conf files on the one we have for report_tests/option_tests, but I can clean them up by removing the commented options and leaving just what's needed for the tests.

I agree, let's clean up the comments, at least that's my opinion

  • I have my doubts about the test location. I have them live inside option_tests because of the cmdline options, but then I decided to move them to a specific new location inside plugins. Does this location make sense?

I don't have an opinion on that, as long as we're testing

  • Any improvements regarding test names are welcome, as always :)

Naming is cool with me

  • Considered adding a test for preset vs config file vs cmdline, to make it more complete. Thoughts on this?

I think that would make sense, then it covers all the options

@jcastill
[tests] Add tests for skip/enable-plugins merge from config and cmdline
15d138a 
Verify that skip-plugins and enable-plugins are merged (not replaced)
when specified in both sos.conf and command line. Also verify that
the option only-plugins uses replacement (not merge) approach,
and that preset options take precedence over config file values.

This is a regression test for commit 8fc655a, which fixed
the merge behavior. The new tests verify both effective options
logging in sos.log and actual plugin execution.

Signed-off-by: Jose Castillo <jcastillo@redhat.com>
@jcastill jcastill force-pushed the jcastillo-add-test-4327 branch from 5a33840 to 15d138a Compare June 11, 2026 12:16
@jcastill

jcastill commented Jun 11, 2026

Copy link
Copy Markdown
Member Author

I've added some extra tests to cover more use cases

@pmoravec

pmoravec commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

The failed stage_one deb tests have always a weird uncaught exception:

sos_logs/hyperv-plugin-errors.txt 
Traceback (most recent call last):
  File "/home/runner/work/sos/sos/sos/sos/report/__init__.py", line 1272, in setup
    plug.setup()
  File "/home/runner/work/sos/sos/sos/sos/report/plugins/hyperv.py", line 21, in setup
    self.add_copy_spec([
  File "/home/runner/work/sos/sos/sos/sos/report/plugins/__init__.py", line 1902, in add_copy_spec
    if file_is_binary(_file):
  File "/home/runner/work/sos/sos/sos/sos/utilities.py", line 190, in file_is_binary
    tfile.read(1)
OSError: [Errno 5] Input/output error

which makes a little sense - what is so wrong on:

        self.add_copy_spec([
            "/sys/bus/vmbus/drivers/",
            # copy devices/*/* instead of devices/ to follow link files
            "/sys/bus/vmbus/devices/*/*"
        ])

? Are there some special files we must avoid ollecting?

@pmoravec

pmoravec commented Jun 12, 2026

Copy link
Copy Markdown
Contributor 
Error: Unable to download artifact(s): Artifact not found for name: sos-22.04.deb

?

Anyway, up to the test failures (that sound irelevant to the PR itself), +1 for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Kind/Testing Related to Testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jcastill @arif-ali @pmoravec